-
Notifications
You must be signed in to change notification settings - Fork 159
feat: allow multiple chart types, add donut, heatmap #7142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
web-common/src/features/canvas/components/charts/circular-charts/CircularChart.ts
Outdated
Show resolved
Hide resolved
Code Review Agent Run Status
|
Code Review Agent Run Status
|
|
I don't necessarily think this should hold anything up, but my impression was that we wanted to default to a donut chart visualization rather than a pie chart, which would mean our default inner radius should be something other than 0. I also think inner radius is maybe the wrong control to expose. It should be relative to the outer radius of the pie chart, which is not user controllable and determined responsively by the size/positioning of the widget. It's more a "thickness" property that goes from, you know, 10% to 100%. You can see the issue when you set the inner radius to something large and then resize the widget container to something small. |
This makes sense. Vega doesn't support percentage values for radius out of the box but I think there's a way to have relative inner radius using expression functions. I will take this as a follow up so that this and the legend PR are unblocked. |
Checklist: